fix: Form collection, multiple dragging and dropping of parameters failed(#3745)#3874
fix: Form collection, multiple dragging and dropping of parameters failed(#3745)#3874wangdan-fit2cloud merged 1 commit intov1from
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| onDragHandle() | ||
| }) | ||
| </script> | ||
| <style lang="scss" scoped></style> |
There was a problem hiding this comment.
The provided code looks largely correct, but there are a few suggestions and improvements to consider:
-
Cloning
form_data.value.form_field_list:
Since you're modifying an array directly (which can lead to unintended side effects), cloning it before performing operations is generally a good practice. However, this might be more relevant within the context of larger applications where multiple components might access the same data. -
Using
map()for Array Operations:
The current implementation usesfilter, which creates a new filtered list instead of modifying the original. If you intend to perform modifications that change the order or remove elements, usingmap()would better fit the intent. -
Consistent Variable Naming:
Ensure variable names likeitems.oldIndex,movedItem`, etc., are consistent with standard naming conventions. -
Handling Edge Cases:
Consider adding checks to ensure thatevt.oldIndexandevt.newIndexfall within valid bounds.
Here's a refined version of the code incorporating these suggestions:
const sync_form_field_list = () => {
const fields = [
{ /* ... */ },
{ /* ... */ }
]
set(props.nodeModel.properties.config, 'fields', fields)
props.nodeModel.clear_next_node_field(false)
onDragHandle()
}
const addFormCollectRef = ref<InstanceType<typeof AddFormCollect>>()
const editFormCollectRef = ref<InstanceType<typeof EditFormCollect>>()
function onDragHandle() {
onEnd: (evt) => {
if (evt.oldIndex === undefined || evt.newIndex === undefined) return
const items = cloneDeep(form_data.value.form_field_list) // Clone the array first
if (
evt.oldIndex <= items.length - 1 &&
evt.newIndex >= 0 &&
evt.newIndex <= items.length - 1
) {
const movedItem = items.splice(evt.oldIndex, 1)[0];
items.splice(evt.newIndex, 0, movedItem);
form_data.value.form_field_list = items;
// Optionally emit event(s) here based on your requirements
} else {
console.warn(`Invalid drag index. Old Index: ${evt.oldIndex}, New Index: ${evt.newIndex}`);
}
},
})
onMounted(() => {
set(props.nodeModel, 'validate', validate)
sync_form_field_list();
props.nodeModel.graphModel.eventCenter.emit('refresh_incoming_node_field');
})These changes should help improve readability and robustness, especially in larger-scale projects.
2 participants
fix: Form collection, multiple dragging and dropping of parameters failed(#3745)